Fix legend border artifact in ppc_freqpoly#537
Fix legend border artifact in ppc_freqpoly#537utkarshpawade wants to merge 7 commits intostan-dev:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a ggplot2 legend rendering artifact in the posterior predictive check frequency polygon plots (ppc_freqpoly() and, transitively, ppc_freqpoly_grouped()), where the geom_area() outline color was bleeding into the legend key.
Changes:
- Add a
guides(fill = guide_legend(override.aes = list(color = NA)))override to suppress the outline color in the fill legend key forppc_freqpoly(). - Document the fix in
NEWS.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
R/ppc-distributions.R |
Overrides legend aesthetics for the fill guide to remove the unwanted border artifact in freqpoly PPC plots (also affects grouped variant via delegation). |
NEWS.md |
Adds a changelog entry describing the legend border artifact fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #537 +/- ##
=======================================
Coverage 99.18% 99.18%
=======================================
Files 35 35
Lines 6118 6119 +1
=======================================
+ Hits 6068 6069 +1
Misses 50 50 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jgabry
left a comment
jgabry
left a comment
There was a problem hiding this comment.
Thanks! I made one review comment. Also, do we have the same issue with ppd_freqpoly or just the ppc_ version?
|
No, ppd_freqpoly doesn't have the issue, it calls legend_none() (ppd-distributions.R:283) since there's no y vs yrep distinction to show. The legend-border artifact is specific to ppc_freqpoly/ppc_freqpoly_grouped. |
|
Thinking about this a bit more, maybe we actually want the border in the plot? For example, with low alpha levels y and yrep can look similar except for the outline. This PR would remove the outline. What do you think? |
|
I checked this and guide_legend(override.aes = list(color = NA)) only affects the legend key glyph, not the plot geoms. The geom_area() outlines in the plot are untouched, so the low-alpha distinguishability between y and yrep you mentioned is preserved. Before:legend key has a dark border box around the y glyph: After:legend key is clean; plot polygons still have their outlines. |
|
Ok thanks. However, now I'm also wondering why we would actually want to remove the border in the legend. The border is there because the plot also has a border. So isn't the legend more accurate if it includes the border? |
|
yes, you're right Closing this PR as working-as-intended. |
|
Ok thanks. Also see here if you haven't seen it yet: #545 |
Fixes #134
ppc_freqpoly()andppc_freqpoly_grouped()displayed a visible dark border around theylegend key due to thecoloraesthetic fromgeom_area()bleeding into the legend glyphguides(fill = guide_legend(override.aes = list(color = NA)))to suppress the outline color in legend keysBefore
After
